Refactor decompose for global functions #793
Conversation
|
Ok so it turned out to be quite some more work that I expected it to be, because of the way we handle objectives, but it should be ready for review now. Currently, the new element decomposition is not the linear-friendly one, as it does not work when the array we're indexing contains integer variables. sum((i == idx) * arr[i]) We can change this easily later now, but first #769 needs to merged. |
| def decompose_comparison(self): | ||
| return [self.args[0] + self.args[1]] # your decomposition | ||
| def decompose(self): |
There was a problem hiding this comment.
I think this whole toplevel doc deserve a slight update.
E.g. instead of "then preferably define it with a standard comparison decomposition,"
something like
"then implement a decompose function that returns a tuple of two arguments:
- a single expression representing what the global function computes (this is often a sum over auxiliary variables, or a single auxiliary variable)
- a list of constraints that define the auxiliary variables used in the first argument
To make maximum use of simplification and common subexpression elimination, we recommend that you use nested expression as much as possible and avoid creating auxiliary variables unless really needed"
There was a problem hiding this comment.
Below in the abstract class, you write "Returns a decomposition into smaller constraints.
Returns a numerical expression and a list of defining constraints."
something consistent with that is also fine : )
cpmpy/expressions/globalfunctions.py
Outdated
| return [eval_comparison(cpm_op, _min, cpm_rhs)], \ | ||
| [cp.any(x <= _min for x in self.args), cp.all(x >= _min for x in self.args), ] | ||
| _min = intvar(*self.get_bounds()) | ||
| return _min, [cp.all(x >= _min for x in self.args), cp.any(x <= _min for x in self.args)] |
There was a problem hiding this comment.
I would find it more intuitive to read it with the order flipped, maybe also vectorized, e.g.
return _min, [cp.all(_min <= self.args), cp.any(_min >= self.args)]
There was a problem hiding this comment.
I flipping the order if probably more intuitive, but we can't do vectorized if the _min is the first arg of the comparison
| assert _abs.lb == 0 | ||
|
|
||
| is_pos = arg >= 0 | ||
| return _abs, [is_pos.implies(arg == _abs), (~is_pos).implies(arg == -_abs)] |
There was a problem hiding this comment.
there are probably LP-friendly ways to do this, which we should keep in mind for linearize later
# Conflicts: # tests/test_globalconstraints.py
As discussed offline, global functions can be decomposed into a numerical value, and some defining constraints.
This PR accomplishes that by removing the
decompose_comparisonfunction we have now.It also massively simplifies the
decompose_in_treecode as there are way fewer cases to check.